fix(authz): block non-admin members from changing duration on published events by JpMaxMan · Pull Request #538 · OpenStackweb/summit-api

JpMaxMan · 2026-05-07T17:18:06Z

Summary

Move the published-event duration guard into SummitEvent::setDuration so the invariant is enforced at the entity boundary, not just in the UI or a single controller.

Why

For a published event, SummitEvent::setDuration → TimeDurationRestrictedEvent::_setDuration silently shifts end_date = start_date + duration whenever start_date is set. The track-chair edit path (PUT /api/v1/summits/{id}/events/{event_id} → OAuth2SummitEventsApiController@updateEvent → AbstractPublishService::updateDuration) calls setDuration(..., $current_member) without checking publication state, which means a chair token + curl can move a live schedule slot with no audit and no notification.

Putting the check in the controller would close the chair path but leave the invariant scattered: any future service or controller that calls setDuration would need to remember to repeat the check. The setter is the single place every caller must pass through, so the entity should own the rule.

What changed

app/Models/Foundation/Summit/Events/SummitEvent.php — setDuration now throws ValidationException when the event is published and the supplied $member is non-admin for the summit.
tests/SummitEventModelTest.php — adds two cases against an already-published presentation fixture: a mocked non-admin member is rejected, a mocked admin member succeeds and the end_date shifts as before.

Compatibility notes

The check intentionally skips when $member is null. Trusted internal callers like SummitService::advanceSummit (app/Services/Model/Imp/SummitService.php:2731) call setDuration with no member during bulk schedule shifts and must continue to work. That mirrors the convention used elsewhere where authorization checks are gated on a user being present.
The existing testChangeEventDuration continues to pass because it also calls setDuration without a member.
This is the API-side complement to the UI gate landed in fntechgit/track-chairs#67. They are independent — either alone closes the visible workflow, but the entity guard is what protects against direct API calls.

Test plan

phpunit tests/SummitEventModelTest.php — new tests pass; existing tests still pass.
Manual: as a track chair, PUT /api/v1/summits/{id}/events/{published_event_id} with {"duration": 1800} returns a 412 ValidationException response (was: 200 with the slot's end_date silently moved).
Manual: as a summit admin, the same call still succeeds.
Manual: scheduling/advance-summit bulk operations continue to update durations on published events.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

Enhanced authorization controls for published summit event duration modifications. The system now validates member permissions before allowing duration changes on published events. Non-authorized members cannot modify published event durations and will receive validation errors on attempt. This prevents unauthorized schedule changes while allowing authorized members to adjust event times as needed.

…ed events SummitEvent::setDuration delegates to _setDuration which, when start_date is set, silently shifts end_date to keep start_date + duration aligned. For an event that has already been published to the schedule, this means any non-admin caller (e.g., a track chair hitting PUT /events/{id} with {duration} via AbstractPublishService::updateDuration) could move a live slot with no audit, no notification, and no validation that the move is intended. Add the invariant at the entity setter so all callers are bound by the same rule, regardless of which controller or service initiated the call. The check intentionally skips when no member is supplied so trusted internal callers (e.g., SummitService::advanceSummit during bulk schedule shifts) continue to work. Pairs with the track-chairs UI gate in fntechgit/track-chairs#67. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-05-07T17:18:24Z

📝 Walkthrough

Walkthrough

This PR adds authorization validation to SummitEvent::setDuration(). When a member attempts to change duration on a published event, the method now checks if that member is allowed for the summit. Non-allowed members receive a ValidationException; allowed members proceed normally. Test coverage validates both authorization paths.

Changes

Published Event Duration Authorization

Layer / File(s)	Summary
Core Authorization Logic `app/Models/Foundation/Summit/Events/SummitEvent.php`	`SummitEvent::setDuration()` adds a guard that throws `ValidationException` if a published event is modified by a member not allowed for the summit.
Test Coverage & Imports `tests/SummitEventModelTest.php`	Test file imports `ValidationException` and adds two test cases: one verifying non-allowed member raises exception, another verifying allowed member updates duration and advances end date.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit hops through authorization's gate,
Checking members swift, not letting them wait—
Published events now guarded with care,
Only summit-allowed may dare
To change what duration must bear! 🐰✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(authz): block non-admin members from changing duration on published events' directly and accurately reflects the main change: adding authorization checks to prevent non-admin members from modifying published event durations.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/track-chair-cannot-modify-published-duration

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

Generate code and open pull requests
Plan features and break down work
Investigate incidents and troubleshoot customer tickets together
Automate recurring tasks and respond to alerts with triggers
Summarize progress and report instantly

Built for teams:

Shared memory across your entire org—no repeating context
Per-thread sandboxes to safely plan and execute work
Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2026-05-07T17:20:19Z

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-538/

This page is automatically updated on each push to this PR.

coderabbitai

🧹 Nitpick comments (1)

tests/SummitEventModelTest.php (1)

83-105: ⚡ Quick win

Add a regression test for the null-member compatibility path.

Given the documented behavior, add a test that setDuration(..., null) still succeeds on published events for trusted internal flows; this protects against future hardening regressions.

Proposed test addition

+    public function testNullMemberCanChangeDurationOnPublishedEvent(){
+        $presentation = self::$presentations[0];
+        $this->assertTrue($presentation->isPublished());
+
+        $old_end_date = $presentation->getEndDate();
+        $presentation->setDuration(864000, false, null);
+        $new_end_date = $presentation->getEndDate();
+
+        $this->assertTrue($old_end_date < $new_end_date);
+    }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/SummitEventModelTest.php` around lines 83 - 105, Add a new regression
test method in SummitEventModelTest that mirrors
testAdminMemberCanChangeDurationOnPublishedEvent but passes null for the $member
argument to Presentation::setDuration to exercise the null-member compatibility
path: grab a published presentation (self::$presentations[0]), assert
isPublished(), record old end date, call $presentation->setDuration(864000,
false, null) and assert no exception and that getEndDate() increased (old <
new). Ensure the test name clearly indicates null-member behavior (e.g.,
testNullMemberCanChangeDurationOnPublishedEvent) and follows the same
mocking-free flow as the admin test.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/SummitEventModelTest.php`:
- Around line 83-105: Add a new regression test method in SummitEventModelTest
that mirrors testAdminMemberCanChangeDurationOnPublishedEvent but passes null
for the $member argument to Presentation::setDuration to exercise the
null-member compatibility path: grab a published presentation
(self::$presentations[0]), assert isPublished(), record old end date, call
$presentation->setDuration(864000, false, null) and assert no exception and that
getEndDate() increased (old < new). Ensure the test name clearly indicates
null-member behavior (e.g., testNullMemberCanChangeDurationOnPublishedEvent) and
follows the same mocking-free flow as the admin test.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d22467f-83d1-41a2-962f-a71cc3209245

📥 Commits

Reviewing files that changed from the base of the PR and between 34e47f9 and 0fd1601.

📒 Files selected for processing (2)

app/Models/Foundation/Summit/Events/SummitEvent.php
tests/SummitEventModelTest.php

smarcet

LGTM

romanetar

LGTM

coderabbitai Bot reviewed May 7, 2026

View reviewed changes

smarcet requested review from romanetar and smarcet May 7, 2026 17:31

smarcet approved these changes May 7, 2026

View reviewed changes

romanetar approved these changes May 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(authz): block non-admin members from changing duration on published events#538

fix(authz): block non-admin members from changing duration on published events#538
JpMaxMan wants to merge 1 commit intomainfrom
fix/track-chair-cannot-modify-published-duration

JpMaxMan commented May 7, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 7, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

smarcet left a comment

Uh oh!

romanetar left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

JpMaxMan commented May 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

What changed

Compatibility notes

Test plan

Summary by CodeRabbit

Bug Fixes

Uh oh!

coderabbitai Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

github-actions Bot commented May 7, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

smarcet left a comment

Choose a reason for hiding this comment

Uh oh!

romanetar left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JpMaxMan commented May 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 7, 2026 •

edited

Loading